Skip to content

atelet: cache pool resolutions so a list flap cannot split the CPU counter - #1479

Open
Tim Bai (baizhenyu) wants to merge 1 commit into
agent-substrate:mainfrom
baizhenyu:statspoller-pool-cache
Open

atelet: cache pool resolutions so a list flap cannot split the CPU counter#1479
Tim Bai (baizhenyu) wants to merge 1 commit into
agent-substrate:mainfrom
baizhenyu:statspoller-pool-cache

Conversation

@baizhenyu

@baizhenyu Tim Bai (baizhenyu) commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

Fixes #1478.

One failed pod list per sweep grouped that tick's samples without ate.workerpool.* labels. Gauges blink and heal; the CPU counter does not: an OTel counter binds increments to their attribute set, so a single flap re-homes that sweep's CPU increase onto a newly-born pool-less series that coexists with the pooled one forever — pool-filtered rate() dips on every flap, a phantom unlabeled series bursts exactly during apiserver trouble, and cardinality doubles per flapped group for the process lifetime.

Fix

A pod's pool is immutable for the pod's lifetime (the ate.dev/worker-pool label is stamped at creation; pods never move between pools), so caching resolutions per pod UID carries zero correctness risk. resolveWorkerPools:

  • lets fresh resolutions win (harmless either way, given immutability),
  • falls back to cachedPools when the list fails outright or misses one pod,
  • rebuilds the cache restricted to the pods whose ateom directories exist this sweep — pruning departed pods and bounding memory with the same prune-to-the-living pattern lastCPU uses.

The residual unlabeled case shrinks to "pod first seen while the apiserver is unreachable", which heals on the next good list — and is pinned by a test as the documented behavior.

Testing

  • List flap: seed the cache, fail the next list, assert the samples stay on the pooled label set with no pool-less split.
  • Prune: an entry whose ateom directory is gone leaves the cache; the live pod's entry stays.
  • Residual case: a pod first seen during an outage groups without pool labels (one group, not an error).
  • hack/verify-all.sh green.

Part of #896, toward #550.

Note: cache lifetime is tied to ateom directories, which nothing cleans up

cachedPools evicts an entry on the first sweep after its ateom directory disappears — but today nothing removes ateoms/<podUID>/ when a worker pod is deleted (the only cleanup anywhere is an ateom clearing its own stale socket at boot; directories accumulate until node replacement). Repo archaeology found no decision behind this: the directory set had no reader until the stats sweep (#961) promoted it into a discovery registry, so GC was never needed before.

Consequences for this PR, sized honestly:

  • Correctness is unaffected. Pod UIDs are never reused, and a retained entry is only consulted when a sample arrives from that UID — a dead socket produces none.
  • Memory is a rounding error on a pre-existing cost. The cache is bounded by the directory count (~150 bytes per departed pod, ~1.5 MB per ten thousand); each stale directory already costs more than its cache entry does — a dial attempt every sweep plus debug-log noise, on main today, independent of this PR.
  • Eviction is deliberately not tied to the pod list — "the list stopped returning this pod" is exactly the flap signal this fix exists to distrust. The filesystem stays the retention authority, consistent with being the discovery authority.

The durable fix is directory GC (atelet holds both halves of the evidence each sweep: the dir set and the node's pod list; "absent from the pod list AND socket dead for N consecutive sweeps → remove" is safe against half-born ateoms and transient list failures). That is a pre-existing issue being filed separately; when it lands, cache eviction follows it automatically with zero changes here.

@baizhenyu Tim Bai (baizhenyu) added kind/bug Something isn't working / bugfixes area/observability bug-bash and removed kind/bug Something isn't working / bugfixes labels Sep 4, 2026
@baizhenyu
Tim Bai (baizhenyu) force-pushed the statspoller-pool-cache branch 2 times, most recently from 8fde844 to 9494dbd Compare September 9, 2026 14:28
…unter

The sweep resolved worker pools with one fresh pod list per tick, and a
failed list grouped that tick's samples without the pool labels. For
the gauges that is a one-tick blink; for ate.actor.stats.cpu.time it is
permanent -- a counter binds every increment to its attribute set, so
one flap re-homes that sweep's CPU increase onto a pool-less series
that then coexists with the pooled one forever: pool-filtered rates dip
on every flap, a phantom unlabeled series bursts exactly during
apiserver trouble, and the split never heals.

A pod's pool is immutable for the pod's lifetime, so caching by pod UID
carries no correctness risk: resolveWorkerPools lets fresh resolutions
win, falls back to cachedPools when the list fails or omits a pod, and
rebuilds the cache restricted to the pods whose ateom directories exist
-- the same prune-to-the-living bound lastCPU keeps. The residual
unlabeled case is a pod no fetch has resolved yet, which heals on the
next list that returns it. A sweep with no ateom directories skips the
fetch entirely.

While here, the raw-list seam is renamed for what it does -- the func
field workerPools read like map access at its call sites -- giving the
pair its vocabulary: fetchWorkerPools (one apiserver list, can fail)
under resolveWorkerPools (answers by whatever means), built by
newWorkerPoolFetcher.

Tests pin each promise separately: flap survival with no label-set
split, the per-pod partial-list fallback (a whole-map fallback passes
the flap test and fails this one), cache pruning to the live directory
set, and the pod-first-seen-during-outage residual.

Fixes agent-substrate#1478. Part of agent-substrate#896, toward agent-substrate#550.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/observability bug-bash kind/bug Something isn't working / bugfixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: one failed pod list re-homes ate.actor.stats.cpu.time increments onto a permanent pool-less series

1 participant